[AIT-676] Move /api/chat default from core to Vercel layer#35
[AIT-676] Move /api/chat default from core to Vercel layer#35lawrence-forooghian wants to merge 1 commit intomainfrom
/api/chat default from core to Vercel layer#35Conversation
/api/chat default from core to Vercel layer
/api/chat default from core to Vercel layer/api/chat default from core to Vercel layer
dfe3364 to
e3729c8
Compare
e3729c8 to
4b71a8f
Compare
0646512 to
75c08db
Compare
75c08db to
dba1ef8
Compare
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| ...options, | ||
| codec: UIMessageCodec, | ||
| // Mirrors the Vercel AI SDK's DefaultChatTransport default. | ||
| api: options.api ?? '/api/chat', |
There was a problem hiding this comment.
this /api/chat string default seems a bit buried now.. should it be some constant in this file or something?
There was a problem hiding this comment.
I'm curious what value you think this would add? It's only used in this place and isn't intended as a reusable value. If you feel strongly I'm happy to change though
There was a problem hiding this comment.
I think it's pretty hidden / unclear what would happen if you don't pass options.api. How would a developer understand the behaviour of the system?
There was a problem hiding this comment.
It's documented on VercelClientTransportOptions:
(defaults to
"/api/chat")
zknill
left a comment
There was a problem hiding this comment.
approach looks good, just a note about where we expose the default url string
dba1ef8 to
37af638
Compare
The default HTTP endpoint "/api/chat" is the Vercel AI SDK's default (set by DefaultChatTransport), but we were setting it in our generic core transport. This violates the two-layer architecture principle that the generic layer should know nothing about Vercel. Make `api` required on core `ClientTransportOptions` and apply the "/api/chat" default in the Vercel layer's `createClientTransport` factory. To avoid forcing useChat users to call `useClientTransport` (where `api` would now be required), `useChatTransport` now returns a `ChatTransportHandle` containing both the `ChatTransport` and the underlying `ClientTransport`. Previously it only returned the `ChatTransport`, so callers had no way to access the transport it created internally — they needed a separate `useClientTransport` call. The handle lets useChat users get the Vercel default without extra configuration, while still having access to the transport for `useMessageSync`, `useActiveTurns`, `useView`, etc. [AIT-676] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
37af638 to
0ee595e
Compare
Note: This is a Claude-generated thing but it reflects the result of my thinking and I've read through it all.
Summary
The default HTTP endpoint "/api/chat" is the Vercel AI SDK's default (set by
DefaultChatTransport), but we were setting it in our generic core transport. This violates the two-layer architecture principle that the generic layer should know nothing about Vercel.Make
apirequired on coreClientTransportOptionsand apply the "/api/chat" default in the Vercel layer'screateClientTransportfactory.To avoid forcing useChat users to call
useClientTransport(whereapiwould now be required),useChatTransportnow returns aChatTransportHandlecontaining both theChatTransportand the underlyingClientTransport. Previously it only returned theChatTransport, so callers had no way to access the transport it created internally — they needed a separateuseClientTransportcall. The handle lets useChat users get the Vercel default without extra configuration, while still having access to the transport foruseMessageSync,useActiveTurns,useView, etc.Options considered
This PR implements Option B below. Feedback welcome on whether Option A would be preferable.
Both diffs are against the Client - React with
useChatsection of the README.Option A: Make
apirequired everywhereSimplest change. Remove the default from core, make
apirequired onClientTransportOptions. All callers must specify it explicitly. No new types or return type changes.import { useChat } from '@ai-sdk/react'; import { useChannel } from 'ably/react'; import { useClientTransport, useActiveTurns, useView } from '@ably/ai-transport/react'; import { useChatTransport, useMessageSync } from '@ably/ai-transport/vercel/react'; import { UIMessageCodec } from '@ably/ai-transport/vercel'; function Chat({ chatId, clientId }: { chatId: string; clientId?: string }) { const { channel } = useChannel({ channelName: chatId }); - const transport = useClientTransport({ channel, codec: UIMessageCodec, clientId }); + const transport = useClientTransport({ channel, codec: UIMessageCodec, clientId, api: '/api/chat' }); const chatTransport = useChatTransport(transport);Pros: Simple implementation, explicit, no new abstractions.
Cons:
useChatusers must specifyapieven though Vercel's own default transport doesn't require it.Option B: Preserve the default in the Vercel layer (this PR)
Make
apirequired in core but optional in the Vercel layer.useChatTransportreturns both theChatTransportand the underlyingClientTransport.import { useChat } from '@ai-sdk/react'; import { useChannel } from 'ably/react'; -import { useClientTransport, useActiveTurns, useView } from '@ably/ai-transport/react'; +import { useActiveTurns, useView } from '@ably/ai-transport/react'; import { useChatTransport, useMessageSync } from '@ably/ai-transport/vercel/react'; -import { UIMessageCodec } from '@ably/ai-transport/vercel'; function Chat({ chatId, clientId }: { chatId: string; clientId?: string }) { const { channel } = useChannel({ channelName: chatId }); - const transport = useClientTransport({ channel, codec: UIMessageCodec, clientId }); - const chatTransport = useChatTransport(transport); + const { chatTransport, transport } = useChatTransport({ channel, clientId });Pros: Matches Vercel's own ergonomics (no endpoint needed), fewer imports, fewer lines.
Cons: Was there perhaps some motivation that I've missed — perhaps didactic, perhaps something else? — for requiring users to create the
ClientTransportexplicitly?🤖 Generated with Claude Code